-
-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce an f-specification argument in ltcmd #1525
Conversation
Currently this is a draft: there are some clear open questions to answer before it is ready to merge
|
I'd maybe drop the warning on |
Is it possible in the architecture of Cf. the handling of the optional argument in |
As for the argument name, what about an upper case |
88e37f9
to
d55f3ba
Compare
Thinking here is that reading the arg. spec. should be clear: collecting an environment will allow newlines, so should be |
I thought about that - but the pattern we have at the moment is all uppercase specs relate to the matching lowercase in that they take an extra |
Whilst we don't currently do this, it would be possible to do a 'pre-scan' to establish if special settings are needed. The issue I think is that unlike say |
that pattern is strong and easy to understand so I don't think we should break it, thus that would rule |
Maybe a dumb Saturday evening idea, but what about using a special sort of processor? Right now |
I would think this is bound to produce problems forever. Besides, what you do with respect to chars being |
I see the idea, but we have a few things that mean I suspect it's not quite right
However, it does suggest something we could do - see parallel reply. |
I think @Skillmon's approach could work, with a logic for seeking an optional arg:
This could be done by adding an additional specifier (lets say |
Performance is not that bad, it's just |
I don't think we can use a group, and we have to deal with looping for spaces, but yes, I guess you are broadly right about performance. |
Good point about multiple |
One might well argue that both |
Of course you can use a group! You're doing right now for the
Yes, applying
My argumentation is that |
I think I'll implement the suggestion, we can look at it then decide if we need automation - that would mainly sit in a different part of the code so would still require the underlying |
d55f3ba
to
4193145
Compare
79ff942
to
4600c98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some typos. Only read the non-code part.
4600c98
to
c152931
Compare
I've made two adjustments here:
The second idea is partly-done. At present, you have to turn this on using a |
We could make life a little easier e.g. by saying that optional args before |
Other than the basic question 'is this the right letter', I think we are good-to-go here now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only a very shallow review. But I'm still not happy about using "f" as I wrote. Otherwise only minor comments
31009a3
to
84c6669
Compare
3b6bc66
to
8e1bb89
Compare
I'll continue collecting typos here (because here I conveniently see what was added in this PR). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only some more minor typos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there are a few somewhat, or very, old comments (not by me) here and it is unclear whether they are now resolved.
I have added mostly just a a few agreement emojis, and the "usual grammar corrections/suggestions".
I note that this has, as usual, already been approved so I cannot make any strong requests in this review. Ho-hum!
Neither @davidcarlisle nor @muzimuzhi seem to have reviewed this yet. |
I pinged a group of people to get some feedback - we'd taken a while and I felt most ideas were resolved, but needed to be sure about the implementation. So I was not expected all reviews to be 'in' before merged - we have time before a release for adjustment. |
See latex3/latex3#591.
Internal housekeeping
Status of pull request
Checklist of required changes before merge will be approved
\changes
entries in source includedchanges.txt
updatedltnewsX.tex
(and/orlatexchanges.tex
) updated